fix: DraftEditor crashes in Safari when IME composition committed #2
Conversation
|
Could u elaborate more on why the issue is platform dependent? |
| @@ -192,7 +192,6 @@ class DraftEditorContents extends React.Component<Props> { | |||
| preventScroll, | |||
| selection, | |||
| tree: editorState.getBlockTree(key), | |||
| key: `${key}-${blockKeyMap.get(key) || '0'}`, | |||
There was a problem hiding this comment.
Will the key removal have any side-effect on the parent block of the <Component /> ?
There was a problem hiding this comment.
No, it won't.
React uses key to identify the existing element which won't be unmounted. Also, an element and its descendants won't be unmounted only if its type (e.g. div, span) and key keep the same with the previous one. If an element with no key, React only compares its type and index(index in a children list).
Since is the only child of its parent Child, this diff removes the key from here to its parent which I don't think has any side-effect, AFAIK.
When typing using IME at the end of a composition, draft-js will do a full rerender. Which is done by changing the key of DraftEditor component. If we change the key of a react component, the component itself will not remount, but any children inside this component will do. Therefore, children are unmounted and mounted again, which causes editor crash issue in Safari. Since React reconciliation fail to diff previous DOM and current DOM due to some of them are considered as 'not-existing' which can not be removed by the reconsiliation process. Since this patch, key of existing block won't change hence its children won't remount.
|
Do we want to use this patch for the upcoming release? i.e. Drop the v0.11.8-rc.2 |
|
If yes, I would have to downgrade the package version back to v0.11.8-rc.1 on @twreporter/keystone |
|
No, this patch is not ready to be shipped. Please downgrade package version back to v0.11.8-rc.0 (without this patch) on @twreporter/keystone. |
Would we want to use a stable version(v0.11.8) on @twreporter/keystone or just keep it on rc version? |
|
I wouldn't say it is a stable version since we still have an unresolved Safari issue in this PR base on the above comment :
As a public package which claims to support Safari in latest 2 versions, I'd like to bump to v0.11.8 when current issue has been resolved. If no further concerns, please just keep it on rc version in the upcoming release. |
This patch downgrades the package for since twreporter/draft-js#2 is not yet for release.
Summary
When typing using IME at the end of a composition, draft-js will do
a full rerender. Which is done by changing the key of DraftEditor component.
If we change the key of a react component, the component itself
will not remount, but any children inside this component will do.
Therefore, children are unmounted and mounted again, which causes editor
crash issue in Safari. Since React reconciliation fail to diff previous DOM
and current DOM due to some of them are considered as 'not-existing'
which can not be removed by the reconsiliation process.
Since this patch, key of existing block won't change hence its children won't remount.
Test Plan
Open a file in example folder (e.g. media.html).
Make sure the draft editor won't crash after a IME composition is committed, especially in Safari.